Skip to content

Pass requestId to handlers via metadata #358

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

lforst
Copy link
Contributor

@lforst lforst commented Apr 17, 2025

This PR passes the JSON RPC request ID to the resource, tool, and prompt handlers via the RequestHandlerExtra (extra) argument.

Motivation and Context

I am maintaining the Sentry SDK and to provide observability into MCP servers we would like to be tracing requests to the MCP server.
As it stands, the tool, resource and prompt handlers live in the context of the SSE request handler, which makes it impossible to resort to AsyncLocalStorage to associate messages to the server with the handlers.

Passing the message's request ID along to the handlers allows us to correlate the POST message to the handler execution (allowing us to show users traces where the handlers are connected to the POST request). This would be great for error correlation, and also performance monitoring.

How Has This Been Tested?

I ran an MCP server locally and verified that the data is useful. Also added some tests.

Breaking Changes

No. I don't think the RequestHandlerExtra is part of any user-facing "implementations".

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed
    • Not sure if this requires an update somewhere...

Copy link
Contributor

@ihrpr ihrpr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good


expect(receivedRequestId).toBeDefined();
expect(typeof receivedRequestId === 'string' || typeof receivedRequestId === 'number').toBe(true);
expect(result.content[0].text).toContain("Received request ID:");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can just check the full equality as receivedRequestId is known at that point

@ihrpr ihrpr merged commit f417691 into modelcontextprotocol:main Apr 22, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants